Skip to content

created new rule for NRT Authentication Methods Changed for VIP Users#4679

Merged
v-dvedak merged 11 commits intoAzure:masterfrom
samikroy:patch-24
Nov 2, 2022
Merged

created new rule for NRT Authentication Methods Changed for VIP Users#4679
v-dvedak merged 11 commits intoAzure:masterfrom
samikroy:patch-24

Conversation

@samikroy
Copy link
Contributor

created new rule for NRT Authentication Methods Changed for VIP Users

created new rule for NRT Authentication Methods Changed for VIP Users
@samikroy
Copy link
Contributor Author

@vmanojreddy & @aprakash13 - Able to create rule in Sentinel.
Not sure if a column in watchlist is not getting recognized in build validation.
Request your help !

image

@samikroy
Copy link
Contributor Author

samikroy commented May 6, 2022

@vmanojreddy & @aprakash13 - Could you please here !

Copy link
Contributor

@aprakash13 aprakash13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the query looks good. Made a small suggestion that probably needs to be addressed.
Looking at the validation errors and will update.

@v-marimanda
Copy link
Contributor

@aprakash13 Please take a look and provide your approve.

aprakash13
aprakash13 previously approved these changes Jun 28, 2022
Copy link
Contributor

@aprakash13 aprakash13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @samikroy. Sorry for the delay here. I will look into the validation errors and get that addressed before we can merge this.

@samikroy
Copy link
Contributor Author

Thanks @samikroy. Sorry for the delay here. I will look into the validation errors and get that addressed before we can merge this.

Thank you for the update @aprakash13 . Please let me know for any changes needed.
My observation is that it is failing as the watch list is referred and assuming the same to be available as a built schema.

@aprakash13 aprakash13 requested review from a team as code owners July 12, 2022 13:33
@v-spadarthi
Copy link
Contributor

@aprakash13 :Please have a look for validation errors.

3 similar comments
@v-spadarthi
Copy link
Contributor

@aprakash13 :Please have a look for validation errors.

@v-spadarthi
Copy link
Contributor

@aprakash13 :Please have a look for validation errors.

@v-marimanda
Copy link
Contributor

@aprakash13 :Please have a look for validation errors.

@v-marimanda
Copy link
Contributor

@hi @samikroy We wanted to check on the status of PR #4679. PR is pending from more long time. Let us know if any assistance is required for this PR. As Per our standard operating procedures if no response is received in the next 7 business days we will close this PR. Thank you for your cooperation.

@samikroy
Copy link
Contributor Author

samikroy commented Sep 7, 2022

@hi @samikroy We wanted to check on the status of PR #4679. PR is pending from more long time. Let us know if any assistance is required for this PR. As Per our standard operating procedures if no response is received in the next 7 business days we will close this PR. Thank you for your cooperation.

Thank you for your response @v-marimanda.
Seems this need a revisit to the validation rules in the pipiline.
As have used watchlists in NRT rules which is supported.
https://docs.microsoft.com/en-us/azure/sentinel/near-real-time-rules#:~:text=%2C%20refer%20to%20multiple%20watchlists%20and%20to%20threat%20intelligence%20feeds.

@v-mchatla
Copy link
Contributor

Hi @samikroy
Please address the suggestions provided by @oshezaf. If you have already addressed the comments, please request @oshezaf for re review your changes.
Thanks

@samikroy
Copy link
Contributor Author

@oshezaf - Thank you for your suggestion & apologies for the delay in response.
Have update the query .
Request you to have a look
Thank you.

oshezaf
oshezaf previously approved these changes Sep 28, 2022
Copy link
Contributor

@oshezaf oshezaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at least as far as my comment goes.

@samikroy
Copy link
Contributor Author

Approved at least as far as my comment goes.

Thank you @oshezaf .
@v-mchatla - Could you please help merge this rule.

@v-mchatla
Copy link
Contributor

Hi @samikroy
All the checks need to be passed to merge the PR. Can you please fix the validation failures.
image

@samikroy
Copy link
Contributor Author

Hi @samikroy All the checks need to be passed to merge the PR. Can you please fix the validation failures. image

@v-mchatla - Please have a look at the history of this PR.
Have mentioned in the beginning about watchlist being scanned for schema.

Seems line #132 from this file causing the failure as schema validation is done in the first place before discounting it as a watchlist.
https://github.com/Azure/Azure-Sentinel/blob/master/.script/tests/KqlvalidationsTests/KqlValidationTests.cs

@aprakash13 have mentioned about addressing this earlier.

Do not see the source code for _queryValidator.ValidateSyntax to fix this further.
Please let me know for more inputs.
Thank you.

private void ValidateKql(string id, string queryStr)
        {
            var validationResult = _queryValidator.ValidateSyntax(queryStr);
            var firstErrorLocation = (Line: 0, Col: 0);
            if (!validationResult.IsValid)
            {
                firstErrorLocation = GetLocationInQuery(queryStr, validationResult.Diagnostics.First(d => d.Severity == "Error").Start);
            }

            var listOfDiagnostics = validationResult.Diagnostics;

            bool isQueryValid = !(from p in listOfDiagnostics
                               where !p.Message.Contains("_GetWatchlist") //We do not validate the getWatchList, since the result schema is not known
                               select p).Any();


@v-mchatla
Copy link
Contributor

Hi @samikroy
We will look into it.
Thanks

@v-mchatla v-mchatla self-assigned this Sep 30, 2022
@v-mchatla
Copy link
Contributor

Hi @samikroy,
Can you please share the watchlist being used in the query.
Thanks

@v-mchatla
Copy link
Contributor

Hi @samikroy,
Can you please share the watchlist being used in the query to resolve validation failures.
Thanks

1 similar comment
@v-spadarthi
Copy link
Contributor

Hi @samikroy,
Can you please share the watchlist being used in the query to resolve validation failures.
Thanks

@samikroy
Copy link
Contributor Author

Hi @samikroy, Can you please share the watchlist being used in the query to resolve validation failures. Thanks

@v-mchatla & @v-spadarthi - Please refer this
https://learn.microsoft.com/en-us/azure/sentinel/watchlist-schemas?WT.mc_id=Portal-Microsoft_Azure_SentinelUS#vip-users

@v-mchatla
Copy link
Contributor

Hi @samikroy,
Thanks for sharing the details. I have occupied with other work. I will work on it today and provide you the update.
Thanks

Applied small tweaks to run validations
@v-mchatla
Copy link
Contributor

Hi @samikroy,
I have verified the watchlist and everything is working fine. Checking with my internal team on how to handle this KQL Validations
Thanks

@v-mchatla
Copy link
Contributor

Hi @samikroy,
I'm already working with my internal team, will keep you posted.
Thanks

@v-mchatla
Copy link
Contributor

Hi @samikroy,
To fix the validation error you need to add below details in .script\tests\KqlvalidationsTests\SkipValidationsTemplates.json
{    "id": "29e99017-e28d-47be-8b9a-c8c711f8a903",    "templateName": "NRT_AuthenticationMethodsChangedforVIPUsers.yaml",    "validationFailReason": "The name 'User Principal Name' does not refer to any known column, table, variable or function"  }
Let me know if you need any details.
Thanks

@v-mchatla
Copy link
Contributor

Hi @samikroy,
Please resolve the conflict and also accept license.
Thanks

1 similar comment
@v-mchatla
Copy link
Contributor

Hi @samikroy,
Please resolve the conflict and also accept license.
Thanks

Copy link
Contributor

@v-mchatla v-mchatla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the checks are passed now. Approving the PR.

@v-dvedak v-dvedak merged commit bdf6ccd into Azure:master Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Detection Detection specialty review needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants